Skip to content

Conversation

@trobugno
Copy link
Contributor

@trobugno trobugno commented Nov 16, 2025

Pandora Extensions Settings - Improvement

Added more dialog to help user the creation of custom property, dependency, etc.

Pandora Bug Fixes

Issues Fixed

Issue n.1: Resource Properties Not Displaying in Editor

Symptom: When selecting a resource (e.g., texture) for a property in Pandora's editor, the field remained empty and the error ERROR: Resource file not found: res:// appeared in the console.

Root Causes: Multiple interconnected issues in the resource property handling system:

  1. Empty string handling in parse_value()
  2. Resource validation rejecting string paths
  3. Resource duplication losing resource_path metadata
  4. UI not accessing raw stored values

Issue n.2: Subcategories and Entities Sharing Data (Reference Sharing Bug)

Symptom: When creating a subcategory (e.g., AlchemyRecipes as child of ItemRecipes) and configuring properties, all child entities and the category itself shared the same data. Modifying one entity would modify all others.

Root Cause: In Pandora's inheritance system, get_default_value() returns the same object reference from the parent if there's no override. UI components directly assigned this reference to current_property, causing modifications to be applied to the shared object before an override was created.

Solution: Added duplicate() method to all custom property classes and modified UI components to work with copies instead of direct references.

Files Modified

1. model/types/resource.gd

Changes:

  • Added empty string handling in parse_value() to prevent loading res://
  • Added explicit null handling in write_value()
  • Modified is_valid() to accept both Resource objects and String paths
  • Added String passthrough in write_value() for path values

Code:

func parse_value(variant: Variant, settings: Dictionary = {}) -> Variant:
	if variant is String:
		# Handle empty strings as null to avoid "res://" loading errors
		if variant == "":
			return null
		# Load the resource and handle potential errors
		var resource = load(variant)
		if resource == null:
			push_warning("Failed to load resource: " + variant)
		return resource
	return variant

func write_value(variant: Variant) -> Variant:
	if variant == null:
		return null
	if variant is Resource:
		return variant.resource_path
	# If it's already a string (path), return it as-is
	return variant

func is_valid(variant: Variant) -> bool:
	# Allow null as valid value for optional resources
	# Allow String (resource paths) as they will be converted to Resources
	# Allow Resource objects directly
	return variant == null or variant is Resource or variant is String

2. model/entity.gd - OverridingProperty.get_default_value()

Changes:

  • Modified to use parse_value() for type conversion
  • Fixed Dictionary parameter issue (was passing nil, now passes {})
  • Changed duplication logic from Object to RefCounted to avoid duplicating Resources

Code:

func get_default_value() -> Variant:
	if _parent_entity._property_overrides.has(_property.get_property_name()):
		var value = _parent_entity._property_overrides[_property.get_property_name()]
		if value is PandoraReference:
			return value.get_entity()
		# Parse the value through the property type to handle conversions
		# (e.g., String paths to Resources)
		var parsed_value = _property.get_property_type().parse_value(value, {})
		# Duplicate RefCounted objects (custom classes like Pandora+, etc.)
		# to avoid reference sharing between entities.
		# Don't duplicate Resources (Texture, AudioStream, etc.) as they are shared assets.
		if parsed_value != null and parsed_value is RefCounted and parsed_value.has_method("duplicate"):
			return parsed_value.duplicate()
		return parsed_value
	return _property.get_default_value()

Note: The duplication fix is critical for preventing reference sharing in custom RefCounted classes while preserving Resource integrity.

3. ui/components/properties/resource/resource_property.gd

Changes:

  • Modified to store path as String instead of loaded Resource
  • Added refresh() call after value change
  • Modified refresh() to access raw override values directly

Code:

func _ready() -> void:
	refresh()
	resource_picker.focus_exited.connect(func(): unfocused.emit())
	resource_picker.focus_entered.connect(func(): focused.emit())
	resource_picker.resource_changed.connect(
		func(resource_path:String):
			# Store the path as string, not the loaded resource
			# The type system will handle conversion via parse_value/write_value
			_property.set_default_value(resource_path)
			refresh()
			property_value_changed.emit(resource_path))

func refresh() -> void:
	if _property != null:
		# Get the raw value from overrides to avoid parse_value conversion
		var raw_value = null
		if _property is PandoraEntity.OverridingProperty:
			var prop_name = _property.get_property_name()
			if _property._parent_entity._property_overrides.has(prop_name):
				raw_value = _property._parent_entity._property_overrides[prop_name]

		if raw_value == null:
			raw_value = _property.get_default_value()

		# Handle both String paths and Resource objects
		if raw_value is String and raw_value != "":
			resource_picker.set_resource_path(raw_value)
		elif raw_value is Resource and raw_value.resource_path != "":
			resource_picker.set_resource_path(raw_value.resource_path)
		else:
			resource_picker.set_resource_path("")

Rationale: Accessing raw values prevents parse_value() conversion which can cause Resources to lose their resource_path metadata due to Godot's resource caching behavior.

4. ui/components/resource_picker/resource_picker.gd

Changes:

  • Added empty path handling in set_resource_path()

Code:

func set_resource_path(path: String) -> void:
	# Handle empty path - clear the field
	if path == "":
		line_edit.text = ""
		self.resource_path = ""
		return

	var resource = load(path) as Resource
	if resource != null:
		line_edit.text = path
		self.resource_path = path

Technical Explanation

The Resource Path Problem

When a Resource is loaded via load() and then retrieved via get_default_value(), Godot's resource caching system may return a Resource instance with an empty resource_path. This happens because:

  1. Resources are cached by Godot's resource loader
  2. Cached resources may not preserve all metadata
  3. The resource_path property can be empty for cached instances

The Solution: String as Source of Truth

Instead of storing the loaded Resource object, we store the path as a String and convert it to a Resource only when needed:

  1. Editor Storage: Path stored as String in _property_overrides
  2. Editor Display: UI reads the String directly without conversion
  3. Runtime Usage: parse_value() converts String → Resource when game code accesses it
  4. Disk Persistence: write_value() ensures String format for serialization

This approach ensures the path is always available for display while still providing proper Resource objects to game code.

Impact

These fixes resolve:

  • ✅ Empty resource fields in editor
  • res:// loading errors
  • ✅ Resource properties not persisting across sessions
  • ✅ Resource duplication issues
  • ✅ Proper handling of null/empty resource values

Backward Compatibility

The changes are backward compatible:

  • Existing properties storing Resources will be converted to paths via write_value()
  • Existing properties storing paths will continue working as before
  • The is_valid() change accepts both formats

@bitbrain
Copy link
Owner

Thank you for raising this. I will check this out locally and play around with it.

trobugno added a commit to trobugno/pandora_plus that referenced this pull request Nov 30, 2025
### What's new?
- Created **configuration.json** following the new **Custom Extensions
Settings** introduced by me in Pandora addon.
- Renamed all property types adding the suffix **_property** to aligned
them with new settings.
- Done a fixes to autoloads when the plugin is enabled.

### Notes
- This build uses an unapproved version of the Pandora addon. To view
the PR I created, follow the
[link](bitbrain/pandora#230)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants